Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add safety preconditions to alloc/src/alloc.rs #118

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

tautschnig
Copy link
Member

These changes add code contracts in the form of #[requires(...)] attributes to various methods in the Global allocator implementation. These contracts ensure that the safety conditions mentioned in the comments are explicitly stated as preconditions for the functions.

These changes add code contracts in the form of `#[requires(...)]` attributes to various methods in the `Global` allocator implementation. These contracts ensure that the safety conditions mentioned in the comments are explicitly stated as preconditions for the functions.
Just as previously done for core: this will enable future use of
`safety::{ensures,requires}` in those crates.
@tautschnig
Copy link
Member Author

How does one do contracts for traits? kani complains that it cannot find any of the contracts that have been added to functions in unsafe impl Allocator for Global (when those in impl Global appear to be fine).

@zhassan-aws
Copy link

AFAIK, this is currently not supported: model-checking/kani#1997

We have a test that is disabled till support is added: https://github.com/model-checking/kani/blob/main/tests/expected/function-contract/trait_impls/methods.rs

Copy link

@celinval celinval left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think all these contracts are missing information about the pointer allocation. The pointer needs to be allocated for the given size for all these operations to be safe.

Also, CI is failing.

library/alloc/src/alloc.rs Outdated Show resolved Hide resolved
Comment on lines +260 to +261
#[requires(layout.size() == 0 || layout.align() != 0)]
#[requires(ptr.as_ptr() as usize % layout.align() == 0)]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't we need to require that the pointer is actually allocated?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We actually need an even stronger precondition (and Kani correctly detects this via the grow_impl harness): we'd need to express that ptr either points to a ZST (that's doable) or is a valid heap allocation (I don't know how we could currently express this).

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We would need ghost state. 😄

@tautschnig
Copy link
Member Author

Also, CI is failing.

Yes, see #118 (comment).

@tautschnig tautschnig self-assigned this Nov 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants